Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of account history #128

Merged
merged 10 commits into from
Dec 15, 2020
Merged

Better handling of account history #128

merged 10 commits into from
Dec 15, 2020

Conversation

bvbfan
Copy link
Contributor

@bvbfan bvbfan commented Dec 3, 2020

This a WIP, i have plan to store rewards by pool id as well, should we have it for account history as well? Now we have owner <-> token <-> amount, but if it's not a pool account we can use token id instead?

@monstrobishi monstrobishi changed the title Better handling of account history [WIP] Better handling of account history Dec 3, 2020
@monstrobishi
Copy link
Contributor

much nicer can you confirm if this has an impact on performance

@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 3, 2020

It's a bit faster, really small, what's save is serialize/deserialize to stream. I plan to make another class to handle rewards and to store rewards by pool id (in validation.cpp where rewards are redistributed)

@monstrobishi
Copy link
Contributor

@bvbfan is this still a work in progress? you can change the title when you feel it's ready to merge in

@bvbfan bvbfan changed the title [WIP] Better handling of account history Better handling of account history Dec 8, 2020
@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 8, 2020

It's no more a WIP, rewards are two types, Commission and PoolReward they are stored separate from account history.

@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 8, 2020

listaccounthistory rpc call remains unchanged for account history, it has txid and txn the difference is there is no empty txid anymore, it means when it has txid it's account history, Rewards type is split to Commission and PoolReward, also rewards have poolID.
Account:
owner
blockHeight
type (Rewards is not valid type anymore)
txn
txid
amounts
Reward:
owner
blockHeight
type (Commission or PoolReward)
poolID
amounts

@monstrobishi
Copy link
Contributor

monstrobishi commented Dec 8, 2020

this could break the app if the Rewards type does not exist anymore, can we name "PoolReward" back to "Rewards" so that the type still exists, and split commission out

so basically just PoolRewards -> Rewards

@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 9, 2020

@monstrobishi
Copy link
Contributor

@bvbfan after merging the other PR (for including UTXO) this has quite a few conflicts

@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 10, 2020

Rebased.

@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 11, 2020

All unit tests pass locally, changes not affect wallet at all.

Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
Signed-off-by: Anthony Fieroni <[email protected]>
@bvbfan
Copy link
Contributor Author

bvbfan commented Dec 14, 2020

@Bushstar can you test new approach?

@monstrobishi monstrobishi merged commit 36c7cd6 into DeFiCh:master Dec 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants